Make the API async where needed, avoid block_on #926
Draft
tnull wants to merge 5 commits into
Draft
Conversation
|
👋 Hi! This PR is now in draft status. |
benthecarman
reviewed
Jun 8, 2026
| node_b.node_id(), | ||
| preimage, | ||
| None, | ||
| ); |
|
|
||
| let liquidity_source = Arc::clone(&liquidity_source); | ||
| let invoice = self.runtime.block_on(async move { | ||
| let invoice = async move { |
Contributor
There was a problem hiding this comment.
do we still need this async move?
| } | ||
| }; | ||
|
|
||
| if let Err(e) = self.runtime.block_on(self.update_payment_store(events)) { |
Contributor
There was a problem hiding this comment.
do we need an async version of these traits in LDK?
| let wallet = self.clone(); | ||
| let runtime = Arc::clone(&self.runtime); | ||
| let logger = Arc::clone(&self.logger); | ||
| runtime.spawn_background_task(async move { |
Contributor
There was a problem hiding this comment.
this seems dangerous to not return an error if persists fails. Idk if we can really get around it though
| let logger = Arc::clone(&listening_logger); | ||
| let listening_addrs = listening_addresses.clone(); | ||
| let listeners = self.runtime.block_on(async move { | ||
| let listeners = async move { |
Contributor
There was a problem hiding this comment.
dont really need this async move
| return Err(Error::NotRunning); | ||
| pub async fn stop(&self) -> Result<(), Error> { | ||
| { | ||
| let mut is_running_lock = self.is_running.write().expect("lock"); |
Contributor
There was a problem hiding this comment.
should we still be using a sync lock for is_running?
joostjager
reviewed
Jun 9, 2026
joostjager
left a comment
joostjager
left a comment
Contributor
There was a problem hiding this comment.
This looks great. But please don't finalize this as a single big PR, but do it incrementally instead.
| Ok(tx) | ||
| fn get_new_address_sync(&self) -> bitcoin::Address { | ||
| let address = self.get_new_address_inner(); | ||
| self.spawn_persist_wallet(); |
Contributor
There was a problem hiding this comment.
Is it safe, return before persist?
| handle.spawn_blocking(func) | ||
| } | ||
|
|
||
| pub fn block_on<F: Future>(&self, future: F) -> F::Output { |
Avoid nested runtime waits when peer, payment, and node metrics operations persist through async storage. This lets callers already in async contexts await those operations directly while keeping existing sync boundaries explicit. Co-Authored-By: HAL 9000
Resolve VSS schema setup through async initialization on the caller runtime so VSS stores no longer create or retain a Tokio runtime. Co-Authored-By: HAL 9000
Construct PostgreSQL storage on the caller runtime now that store-backed APIs are async, removing the store-owned runtime and shutdown path. Co-Authored-By: HAL 9000
Expose startup, shutdown, builder, wallet, liquidity, and bindings entrypoints as async so store-backed persistence no longer needs local synchronous future driving. Update tests and benches to await the async surface and keep store persistence helpers on the async storage path. Co-Authored-By: HAL 9000
Describe the Tokio runtime expectations for LDK Node callers so async API users know when runtime handles are reused and why runtime worker threads must remain available for node progress. Co-Authored-By: HAL 9000
tnull
force-pushed
the
2026-06-async-kvstore-persistence-async-api
branch
from
6a3db1c to
f58384d
Compare
Collaborator
Author
|
Rebased after #919 landed, though we'll probably wait with moving this forward until after we branched-off for v0.8. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #919.
WIP